feat: add support for Polygon (POL) network#147
Merged
Conversation
This change enhances clarity and consistency in network handling.
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds ChangesPolygon POL Nomenclature Migration
Estimated Code Review Effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/utils/supportedChains/supportedChains.test.ts (1)
24-47: ⚡ Quick winStrengthen POL migration coverage for renamed metadata fields.
Current assertions don’t verify the renamed display/native fields changed in config (Line 65 and Lines 74-75 in
src/utils/supportedChains/index.ts). Adding those checks will better guard against partial reverts of the POL rename.✅ Suggested test additions
it('should return pol chain info for CHAIN_ID.pol (Polygon PoS mainnet)', () => { - const { id, name, type, currency, explorerUrl } = SUPPORTED_CHAINS[CHAIN_ID.pol]; + const { id, name, type, currency, explorerUrl, label, nativeCurrency } = + SUPPORTED_CHAINS[CHAIN_ID.pol]; expect(id).toBe(CHAIN_ID.pol); expect(name).toBe('pol'); expect(type).toBe('production'); expect(currency).toBe('POL'); expect(explorerUrl).toBe('https://polygonscan.com'); + expect(label).toBe('Polygon (POL)'); + expect(nativeCurrency?.name).toBe('POL'); + expect(nativeCurrency?.symbol).toBe('POL'); }); it('should return pol chain info when accessing via CHAIN_ID.matic (backward-compat alias for chain 137)', () => { - const { id, name, type, currency, explorerUrl } = SUPPORTED_CHAINS[CHAIN_ID.matic]; + const { id, name, type, currency, explorerUrl, label, nativeCurrency } = + SUPPORTED_CHAINS[CHAIN_ID.matic]; expect(id).toBe(CHAIN_ID.pol); expect(name).toBe('pol'); expect(type).toBe('production'); expect(currency).toBe('POL'); expect(explorerUrl).toBe('https://polygonscan.com'); + expect(label).toBe('Polygon (POL)'); + expect(nativeCurrency?.name).toBe('POL'); + expect(nativeCurrency?.symbol).toBe('POL'); });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/utils/supportedChains/supportedChains.test.ts` around lines 24 - 47, Add assertions that verify the renamed display/native metadata fields on the POL entry and its matic alias: after pulling SUPPORTED_CHAINS[CHAIN_ID.pol] (and SUPPORTED_CHAINS[CHAIN_ID.matic]) check the new fields (e.g. displayCurrency and nativeCurrency — use the actual property names used in supportedChains/index.ts) exist and equal the expected values for POL, and update the alias equality test to also assert SUPPORTED_CHAINS[CHAIN_ID.pol].displayCurrency === SUPPORTED_CHAINS[CHAIN_ID.matic].displayCurrency and SUPPORTED_CHAINS[CHAIN_ID.pol].nativeCurrency === SUPPORTED_CHAINS[CHAIN_ID.matic].nativeCurrency so the rename is covered for both access paths.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@src/utils/supportedChains/supportedChains.test.ts`:
- Around line 24-47: Add assertions that verify the renamed display/native
metadata fields on the POL entry and its matic alias: after pulling
SUPPORTED_CHAINS[CHAIN_ID.pol] (and SUPPORTED_CHAINS[CHAIN_ID.matic]) check the
new fields (e.g. displayCurrency and nativeCurrency — use the actual property
names used in supportedChains/index.ts) exist and equal the expected values for
POL, and update the alias equality test to also assert
SUPPORTED_CHAINS[CHAIN_ID.pol].displayCurrency ===
SUPPORTED_CHAINS[CHAIN_ID.matic].displayCurrency and
SUPPORTED_CHAINS[CHAIN_ID.pol].nativeCurrency ===
SUPPORTED_CHAINS[CHAIN_ID.matic].nativeCurrency so the rename is covered for
both access paths.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 1c69f902-1558-4ef1-a422-950c08ef25e8
📒 Files selected for processing (3)
src/utils/network/index.tssrc/utils/supportedChains/index.tssrc/utils/supportedChains/supportedChains.test.ts
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/__tests__/core/verify.pol.test.ts`:
- Line 7: The test depends on a live Polygon RPC via the POL_RPC_URL constant,
which makes CI flaky; update the test to gate or skip live-network behavior when
a specific env flag is not set (e.g., RUN_LIVE_TESTS or POL_RPC_REAL=true).
Modify the file-level constant POL_RPC_URL and the related test block(s) in
src/__tests__/core/verify.pol.test.ts so that if the flag is absent the test
uses a mocked/local RPC or calls test.skip/describe.skip to bypass the live RPC
assertions; ensure the gating is applied for both the POL_RPC_URL usage and the
tests around the verifyPol* test cases that currently rely on lines ~60-75.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 6ed1d8c4-45c7-427b-b167-e7d648d28d14
📒 Files selected for processing (2)
src/__tests__/core/verify.pol.test.tssrc/__tests__/fixtures/fixtures.ts
- Added new fixtures for W3C Transferable Record and OA Token Registry minted on Polygon mainnet. - Updated tests to validate error handling for missing tokenRegistry and tokenNetwork.chainId. - Adjusted test expectations to ensure proper validation of tampered documents. - Refactored existing tests to improve clarity and maintainability.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/token-registry-functions/utils.ts (1)
15-16:⚠️ Potential issue | 🟠 MajorFix gas option handling and error masking in
getTxOptions/isSupportedTitleEscrowFactory
getTxOptionstreats valid explicit0/0ngas inputs as “missing” viaif (!maxFeePerGas || !maxPriorityFeePerGas)and can end up returning{}instead of preserving the caller’s values (lines 15-16 and 25-26). Use nullish checks (== null) to distinguish missing vs real zero.isSupportedTitleEscrowFactoryuses a broadcatcharoundimplementation(), which can mask unrelated RPC/provider failures; preserve the original error (or narrow the catch) instead of always converting to the v5/v4 message.if (!maxFeePerGas || !maxPriorityFeePerGas) { chainId = chainId ?? ((await getChainIdSafe(signer)) as unknown as CHAIN_ID);Suggested fix
- if (!maxFeePerGas || !maxPriorityFeePerGas) { + if (maxFeePerGas == null || maxPriorityFeePerGas == null) { chainId = chainId ?? ((await getChainIdSafe(signer)) as unknown as CHAIN_ID); const gasStation = SUPPORTED_CHAINS[chainId]?.gasStation; if (gasStation) { const gasFees = await gasStation(); - maxFeePerGas = gasFees?.maxFeePerGas ?? 0; - maxPriorityFeePerGas = gasFees?.maxPriorityFeePerGas ?? 0; + maxFeePerGas = gasFees?.maxFeePerGas; + maxPriorityFeePerGas = gasFees?.maxPriorityFeePerGas; } } - if (!maxFeePerGas || !maxPriorityFeePerGas) { + if (maxFeePerGas == null || maxPriorityFeePerGas == null) { return {}; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/token-registry-functions/utils.ts` around lines 15 - 16, The gas-option check in getTxOptions incorrectly treats explicit zero values as missing; replace the falsy checks on maxFeePerGas and maxPriorityFeePerGas (the `if (!maxFeePerGas || !maxPriorityFeePerGas)` logic) with nullish checks (e.g., `== null`) so 0/0n are preserved and chainId/fee lookups only happen when values are truly absent; also ensure the function returns the caller-provided values rather than an empty {}. In isSupportedTitleEscrowFactory, avoid the broad catch around implementation() that masks RPC/provider errors: narrow the catch to only handle the specific “method not found / v4” condition or rethrow non-matching errors (i.e., inspect the caught error before mapping to the v5/v4 message) so unrelated failures surface rather than being converted into the v5/v4 fallback.
♻️ Duplicate comments (1)
src/__tests__/core/verify.pol.test.ts (1)
308-341:⚠️ Potential issue | 🟠 Major | ⚡ Quick win“Structural” OA tests still depend on live Polygon RPC.
Line 308-341 runs even when
RUN_LIVE_TESTSis false, and Line 12’s default public RPC keeps these tests network-bound/flaky in CI.Suggested fix
-const POL_RPC_URL = process.env.POL_RPC || 'https://polygon-bor-rpc.publicnode.com'; +const POL_RPC_URL = process.env.POL_RPC; -describe.skipIf(!OA_POL_MINTED_READY)( +describe.skipIf(!OA_POL_MINTED_READY || !RUN_LIVE_TESTS || !POL_RPC_URL)( 'pol-oa-token-registry-minted — structural (hash + verifier selection)', () => {Also applies to: 12-12
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/__tests__/core/verify.pol.test.ts` around lines 308 - 341, The tests in the describe.skipIf block (named 'pol-oa-token-registry-minted — structural (hash + verifier selection)') still hit a live Polygon RPC because verifyDocument is called with POL_RPC_URL by default; change the test guard so these network-bound checks are skipped when RUN_LIVE_TESTS is not set/false. Concretely, update the describe.skipIf predicate (or wrap the verifyDocument calls) to use process.env.RUN_LIVE_TESTS (e.g. describe.skipIf(!process.env.RUN_LIVE_TESTS)) or only pass rpcProviderUrl: POL_RPC_URL when process.env.RUN_LIVE_TESTS === 'true'; adjust calls to verifyDocument(polOaTokenRegistryMinted, { rpcProviderUrl: POL_RPC_URL }) and related tampered test so they don't use POL_RPC_URL unless RUN_LIVE_TESTS is enabled (keep test names like 'OpenAttestationHash' and 'OpenAttestationEthereumTokenRegistryStatus' unchanged).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/__tests__/core/verify.pol.test.ts`:
- Around line 376-382: The test 'DOCUMENT_STATUS should be SKIPPED when no
credentialStatus' currently returns early if polW3cVerifiableDocument already
has credentialStatus, making the assertion a no-op; instead, create a copy of
polW3cVerifiableDocument (e.g., const doc = { ...polW3cVerifiableDocument }) and
explicitly delete or set doc.credentialStatus = undefined before calling
verifyDocument(doc), then assert that all fragments with type 'DOCUMENT_STATUS'
have status 'SKIPPED' using verifyDocument and the existing statusFragments
filtering; remove the early return so the test always performs the assertion.
In `@src/__tests__/fixtures/pol-w3c-verifiable-document.json`:
- Around line 44-52: The fixture incorrectly marks a non-transferable document
as transferable by setting credentialStatus.type to "TransferableRecords" and
including tokenNetwork/tokenRegistry/tokenId metadata; update the fixture so
credentialStatus either is removed or replaced with a non-transferable
representation (e.g., omit TransferableRecords and token* fields) to match the
non-transferable test intent, ensuring references to credentialStatus,
TransferableRecords, tokenNetwork, tokenRegistry, and tokenId are corrected
accordingly.
In `@src/token-registry-functions/utils.ts`:
- Around line 95-102: The current bare catch around calling
titleEscrowFactoryContract.implementation() masks all errors and always throws a
misleading "does not expose implementation()" message; update the catch to
inspect the caught error from the implementation() call (the thrown object from
ethers) and only translate it into the factory-missing-selector message when it
clearly indicates an ABI/selector/missing-function situation (e.g., message/code
indicating "missing revert", "method not found", "does not contain function", or
similar ABI-mismatch text), otherwise rethrow the original error (preferably by
throwing a new Error that includes the original error as the cause). Keep
references to titleEscrowFactoryContract.implementation(), factoryAddress and
implAddr when updating the logic so the specific ABI-mismatch is handled while
network/RPC/decoding errors bubble up with their original details.
---
Outside diff comments:
In `@src/token-registry-functions/utils.ts`:
- Around line 15-16: The gas-option check in getTxOptions incorrectly treats
explicit zero values as missing; replace the falsy checks on maxFeePerGas and
maxPriorityFeePerGas (the `if (!maxFeePerGas || !maxPriorityFeePerGas)` logic)
with nullish checks (e.g., `== null`) so 0/0n are preserved and chainId/fee
lookups only happen when values are truly absent; also ensure the function
returns the caller-provided values rather than an empty {}. In
isSupportedTitleEscrowFactory, avoid the broad catch around implementation()
that masks RPC/provider errors: narrow the catch to only handle the specific
“method not found / v4” condition or rethrow non-matching errors (i.e., inspect
the caught error before mapping to the v5/v4 message) so unrelated failures
surface rather than being converted into the v5/v4 fallback.
---
Duplicate comments:
In `@src/__tests__/core/verify.pol.test.ts`:
- Around line 308-341: The tests in the describe.skipIf block (named
'pol-oa-token-registry-minted — structural (hash + verifier selection)') still
hit a live Polygon RPC because verifyDocument is called with POL_RPC_URL by
default; change the test guard so these network-bound checks are skipped when
RUN_LIVE_TESTS is not set/false. Concretely, update the describe.skipIf
predicate (or wrap the verifyDocument calls) to use process.env.RUN_LIVE_TESTS
(e.g. describe.skipIf(!process.env.RUN_LIVE_TESTS)) or only pass rpcProviderUrl:
POL_RPC_URL when process.env.RUN_LIVE_TESTS === 'true'; adjust calls to
verifyDocument(polOaTokenRegistryMinted, { rpcProviderUrl: POL_RPC_URL }) and
related tampered test so they don't use POL_RPC_URL unless RUN_LIVE_TESTS is
enabled (keep test names like 'OpenAttestationHash' and
'OpenAttestationEthereumTokenRegistryStatus' unchanged).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 7c60a4f0-f7d6-462c-bc57-9f1ff6a85bd8
📒 Files selected for processing (7)
src/__tests__/core/verify.pol.test.tssrc/__tests__/fixtures/pol-oa-token-registry-minted.jsonsrc/__tests__/fixtures/pol-w3c-transferable-record-minted.jsonsrc/__tests__/fixtures/pol-w3c-verifiable-document.jsonsrc/token-registry-functions/utils.tssrc/utils/supportedChains/index.tssrc/utils/supportedChains/supportedChains.test.ts
✅ Files skipped from review due to trivial changes (1)
- src/tests/fixtures/pol-oa-token-registry-minted.json
🚧 Files skipped from review as they are similar to previous changes (2)
- src/utils/supportedChains/supportedChains.test.ts
- src/utils/supportedChains/index.ts
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/utils/supportedChains/supportedChains.test.ts (1)
77-119:⚠️ Potential issue | 🟠 Major | ⚡ Quick winStructural/offline tests are incorrectly skipped unless live tests are enabled.
These blocks are labeled structural/offline, but
describe.skipIf(!RUN_LIVE_TESTS)prevents them from running in normal CI. This hides regressions in non-live verification paths.Suggested change
- describe.skipIf(!RUN_LIVE_TESTS)( + describe( 'amoy-oa-token-registry-minted — structural (hash + verifier selection)', () => { ... }, ); - describe.skipIf(!RUN_LIVE_TESTS)( + describe( 'amoy-w3c-transferable-record-minted — structural (offline)', () => { ... }, );Also applies to: 172-202
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/utils/supportedChains/supportedChains.test.ts` around lines 77 - 119, The structural/offline test blocks are incorrectly wrapped with describe.skipIf(!RUN_LIVE_TESTS) which skips them in normal CI; change those blocks (where SUPPORTED_CHAINS is validated) to run unconditionally by replacing describe.skipIf(!RUN_LIVE_TESTS) with a plain describe(...) (or another unconditional test wrapper), leaving any true live-only blocks still guarded by RUN_LIVE_TESTS; ensure you update every occurrence (including the other block noted) so structural/offline checks always execute.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/__tests__/core/verify.amoy.test.ts`:
- Around line 77-119: The test suite uses describe.skipIf(!RUN_LIVE_TESTS) which
skips offline/structural tests when RUN_LIVE_TESTS is false; change the
structural/offline describe blocks (the one titled
'amoy-oa-token-registry-minted — structural (hash + verifier selection)' and the
other at lines ~172-202) to use a plain describe (or a dedicated skip flag for
live-only tests) so they run in CI by default; update the calls around
describe.skipIf and keep the individual live/network-dependent it blocks using
RUN_LIVE_TESTS or conditional skips only where network RPC (verifyDocument with
rpcProviderUrl/AMOY_RPC_URL) is required, leaving pure structural tests
(OpenAttestationHash, tamper check, chainId decode) executed unconditionally;
look for symbols describe.skipIf, RUN_LIVE_TESTS, verifyDocument,
amoyOaTokenRegistryMinted and adjust the describe wrappers accordingly.
- Around line 9-10: The RUN_LIVE_TESTS flag is being set with truthy coercion
(const RUN_LIVE_TESTS = !!process.env.RUN_LIVE_TESTS) which treats values like
"false" as true; change the assignment to explicitly compare the environment
string to "true" (e.g., const RUN_LIVE_TESTS = process.env.RUN_LIVE_TESTS ===
'true') so only the literal "true" enables live tests; update any test logic
that relies on RUN_LIVE_TESTS to continue using that constant.
In `@src/utils/supportedChains/supportedChains.test.ts`:
- Around line 67-71: The test making a real network HEAD request (the it block
referencing SUPPORTED_CHAINS and CHAIN_ID.amoy) should not run by default;
change the test to either skip or only run when a live/network flag is set
(e.g., process.env.RUN_LIVE_TESTS) or replace the fetch call with a mocked
response using your test framework's mocking utilities so no external DNS/HTTP
call is made; update the test title/comment to reflect it's a gated live check
and keep references to SUPPORTED_CHAINS and CHAIN_ID.amoy so reviewers can find
the location.
---
Outside diff comments:
In `@src/utils/supportedChains/supportedChains.test.ts`:
- Around line 77-119: The structural/offline test blocks are incorrectly wrapped
with describe.skipIf(!RUN_LIVE_TESTS) which skips them in normal CI; change
those blocks (where SUPPORTED_CHAINS is validated) to run unconditionally by
replacing describe.skipIf(!RUN_LIVE_TESTS) with a plain describe(...) (or
another unconditional test wrapper), leaving any true live-only blocks still
guarded by RUN_LIVE_TESTS; ensure you update every occurrence (including the
other block noted) so structural/offline checks always execute.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 66a6a0f0-83ec-47d5-b822-50b073520abf
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (9)
.github/workflows/ci.yml.github/workflows/tests.yml.gitignorepackage.jsonsrc/__tests__/core/verify.amoy.test.tssrc/__tests__/fixtures/amoy-oa-token-registry-minted.jsonsrc/__tests__/fixtures/amoy-w3c-transferable-record-minted.jsonsrc/utils/supportedChains/index.tssrc/utils/supportedChains/supportedChains.test.ts
✅ Files skipped from review due to trivial changes (2)
- package.json
- .gitignore
🚧 Files skipped from review as they are similar to previous changes (1)
- src/utils/supportedChains/index.ts
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
…ted data readiness
…d correct chain ID references
…mproved readability
|
rongquan1
approved these changes
Jun 10, 2026
nghaninn
pushed a commit
that referenced
this pull request
Jun 10, 2026
|
🎉 This PR is included in version 2.14.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This change enhances clarity and consistency in network handling.
Summary
What is the background of this pull request?
Changes
Issues
What are the related issues or stories?
Summary by CodeRabbit
Chores
Bug Fixes
Tests